Skip to content

Various fixes and minor improvements#146

Open
uuf6429 wants to merge 19 commits intoimbo:mainfrom
uuf6429:chore/various-fixes
Open

Various fixes and minor improvements#146
uuf6429 wants to merge 19 commits intoimbo:mainfrom
uuf6429:chore/various-fixes

Conversation

@uuf6429
Copy link
Contributor

@uuf6429 uuf6429 commented Dec 14, 2025

This PR changes a few (somewhat unrelated) minor things.

Refer to the comments for an explanation.

Everything should be backward compatible.

@uuf6429 uuf6429 marked this pull request as ready for review December 14, 2025 07:20
@uuf6429
Copy link
Contributor Author

uuf6429 commented Dec 22, 2025

@christeredvartsen I just rebased this, so I think it's ready now.

The only functional change is in src/Exception/ArrayContainsComparatorException.php - where I changed the newlines. It's not the best, but it shouldn't be a breaking change seeing as it impacts exception messages (end users should never depend on the exact exception message).

Copy link
Member

@christeredvartsen christeredvartsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a bunch for the improvements. I have left a few comments in the review.

@uuf6429 uuf6429 force-pushed the chore/various-fixes branch from 17b75bf to 7c7011b Compare January 24, 2026 15:21
@christeredvartsen
Copy link
Member

The composer-cache-dir step seems to be problematic on windows:

- id: composer-cache-dir
  run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT

@christeredvartsen
Copy link
Member

Also, since composer.lock is no longer committed, we might need to change this:

- id: composer-cache
  uses: actions/cache@v5
  with:
    path: ${{ steps.composer-cache-dir.outputs.dir }}
    key: ${{ runner.os }}-php-${{ matrix.php }}-composer-${{ hashFiles('**/composer.lock') }}

Allows us to use bash instead of powershell on all runners
@uuf6429
Copy link
Contributor Author

uuf6429 commented Jan 29, 2026

The remaining failures are most like because of newline mismatch. It will be clearer when I try it on my machine at home later on today.

@christeredvartsen
Copy link
Member

The remaining failures are most like because of newline mismatch. It will be clearer when I try it on my machine at home later on today.

Sounds good. Perhaps some sort of normalization of newlines before matching might be an improvement?

@uuf6429
Copy link
Contributor Author

uuf6429 commented Jan 29, 2026

I figured out the issue in a private repo.

PHP treats NOWDOC and other strings literally as they are in the source file, so if the source happened to contain garbage characters or CRLF, it keeps it as is.

The problem in this case is git's autocrlf setting that is somehow enabled on the github windows runner - when the files are checked out, git was replacing LF with CRLF (even if internally they were stored with LF).

A small change to .gitattributes should fix it for everyone.


The last remaining issue was about a dev-only requirement for ext-sockets, I solved it like I did for ext-fileinfo.

@uuf6429
Copy link
Contributor Author

uuf6429 commented Feb 13, 2026

@christeredvartsen kind reminder - any chance of getting this merged in soon? I'd like to go further with some proposed changes depending on this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants